IGNITE-29033 SQL Calcite: Support UDF and UDTF Overloading - #13544
IGNITE-29033 SQL Calcite: Support UDF and UDTF Overloading#13544tkalkirill wants to merge 4 commits into
Conversation
|
|
||
| assertEquals(2, schema.getFunctions("OVERLOADED").size()); | ||
| assertEquals(2, schema.getFunctions("OVERLOADED_TABLE").size()); | ||
| assertEquals(1, schema.getFunctions("SQL_EQUIVALENT").size()); |
There was a problem hiding this comment.
I wonder - why ? Behavior is different from java - is it ok ? It can confuse and brings, as minimal, unexpected behavior ? If it calcite related design - plz show me the related links ? And as minimal - i expect it need to be documented somehow ?
There was a problem hiding this comment.
This behavior intentionally differs from Java because overload resolution uses SQL types. Both int and Integer correspond to SQL INTEGER, so Calcite cannot reliably distinguish these overloads and their resolution would depend on registration order.
Therefore, overloads must have different SQL parameter types or parameter order. I’ve also documented this behavior in QuerySqlFunction and QuerySqlTableFunction.
| if (!p.getType(Commons.typeFactory()).equalsSansFieldNames(existingP.getType(Commons.typeFactory()))) | ||
| break; | ||
| for (Function existingFun : getFunctions(name)) { | ||
| if (sameParameters(func.getParameters(), existingFun.getParameters(), typeFactory)) { |
There was a problem hiding this comment.
If you will use non ignoring nullability comparison:
if (!paramType.equalsSansFieldNames(existingParamType))
return false;
Seems you can change behavior as it was in java and reduce further overloading missing errors
There was a problem hiding this comment.
I tried this change and checked SqlUtil.lookupRoutine directly. Both overloads remain candidates for both INTEGER and INTEGER NOT NULL, and reversing the candidate order changes which overload is selected.
The earlier integration test was misleading: both methods had the same Java name, so compilation of the generated Java code could resolve the overload again. It did not prove that SQL resolution distinguishes nullability.
I reverted the change and kept the comparison ignoring nullability. Treating int and Integer as the same SQL signature avoids ambiguous selection and additional rules users would need to remember. This limitation is documented in both annotations.
https://issues.apache.org/jira/browse/IGNITE-29033